BBS: expose the signature header parameter (closes #2)#3
Merged
Conversation
`IBbsCryptoProvider` hardcoded the BBS signature `header` to empty at the FFI boundary, so a consumer could not bind application data into a BBS signature/ proof. This blocks the W3C `bbs-2023` cryptosuite from binding its mandatory- disclosure group, the only thing that stops a holder from dropping an issuer-mandated claim and still passing `VerifyProof`. Add an optional `ReadOnlySpan<byte> header = default` to Sign/Verify/ DeriveProof/VerifyProof and thread it into the already-present FFI header arguments. The native zkryptium-ffi layer already accepts and consumes the header (verified in lib.rs and its Rust tests), so no Rust change is required. Rename the existing `nonce` parameter on DeriveProof/VerifyProof to `presentationHeader`: it is the BBS presentation header (`ph`), distinct from the signature `header`. Default-empty preserves prior behavior for callers that omit it. - ICryptoProvider / DefaultBbsCryptoProvider: header param + ph rename + docs - PublicAPI.Unshipped.txt: record the signature changes - BbsHeaderTests: 8 tests proving the header is committed by Verify and the derived proof, independent of ph, with default-empty back-compat - PRD FR-5, BBS sample (section 8), README + FFI README updated Verified: solution builds with -warnaserror; native-present test leg 703/703; BBS sample exits 0; API coverage OK. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Ships the BBS `header` parameter (issue #2) so downstream consumers (dataproofs-dotnet) can pin a release exposing the binding. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
Author
Automated review — looks good ✅Reviewed the title, description, and full diff. This is a clean, well-scoped change that surfaces the BBS signature What I verified
Notes (non-blocking)
Nice work threading the security rationale (mandatory-disclosure binding) through the PRD and the adversarial-review note. No changes requested. Generated by Claude Code |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #2.
IBbsCryptoProviderhardcoded the BBS signatureheaderto empty at the FFI boundary, so a consumer could not bind application data into a BBS signature/proof. This blocks the W3Cbbs-2023cryptosuite from binding its mandatory-disclosure group — the only thing that stops a malicious holder from dropping an issuer-mandated claim and still passingVerifyProof.This PR adds an optional
headerparameter to all four BBS operations and threads it into the already-present FFI header arguments.Key finding: the native
zkryptium-ffilayer already accepts and consumes the header (verified innative/zkryptium-ffi/src/lib.rsand its Rust tests, which round-trip non-empty headers). So this is a managed-layer-only change — no Rust rebuild, no native redistribution.Changes
ICryptoProvider.cs/DefaultBbsCryptoProvider.cs— addReadOnlySpan<byte> header = default(last param) toSign,Verify,DeriveProof,VerifyProof, threaded into the FFIheader_ptr/lenslot. Default-empty preserves prior behavior.nonce→presentationHeaderonDeriveProof/VerifyProof— it is the BBS presentation header (ph), a value distinct from the signatureheader(issuer-fixed at sign time vs holder-chosen at derive time). Source-compatible for all in-repo (positional) callers.PublicAPI.Unshipped.txt— record the signature changes (8 removed, 8 added).BbsHeaderTests.cs— 8 newNativeFFItests: header round-trip; mismatch fails; a header-bound signature fails under the default empty header; the proof commits the header;headerandpresentationHeaderare independently committed; default-header back-compat.API change
The
nonce → presentationHeaderrename and the added optionalheaderare source-compatible for positional callers (the only kind in-repo). A downstream caller using the named argumentnonce:would need to update. Acceptable at1.0.0-preview.1.Verification
dotnet build NetCrypto.sln -c Release -warnaserror→ clean--filter "Category!=BbsAbsent") → 703/703 passedVerifyand the derived proof.Follow-up (out of scope)
This only surfaces the header. To close
dataproofs-dotnet's tracked dependency, a NetCrypto prerelease must ship so it can pin the new version and implement the conformantbbsHeader = SHA-256(proofHash) ‖ SHA-256(mandatoryHash)binding.🤖 Generated with Claude Code